-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
varnishd: add a feature flag to disable bans in VCL #4168
base: master
Are you sure you want to change the base?
Conversation
Add vcl_ban feature flag to disallow usage of ban() in VCL to prevent a possible DoS scenario in a multi-tenant setup.
I am not against, but is this a generic solution to the problem? Would it not make much more sense to add some sort of cache partitioning where a certain value is forcibly added to the hash and bans? |
I agree that there is probably much more improvements to do in this area, but this is a low hanging fruit that can already help in a lot of use cases, and I think it makes sense to have it. |
Aren't the two ideas a bit different? One removes the ability altogether, the other silos it. Both have merits, but they're different |
I agree with both Walid's and Guiallaume's comments, but having this as a global flag really seems to be quite limited. Next up, someone might want |
I'm -1 on this one. It doesn't seem hard to do it properly (see todays IRC log) |
How about a new Initial default value: Since VCL bans simply exist today, we keep them enabled by default for compatibility, but new features like Then we can add a new option to
FYI I would be against adding an override at the label level. We can keep track of VCL overrides separately for enabled and disabled features (with a simple sanity check of |
@dridi per vcl feature flags sound like a very good idea. I'd like to keep the |
I only mentioned Another example could be "import ... from":
|
This is useful on multi-tenant environments to prevent a single user from wiping the whole cache or other user's cache.